Conversation
|
Thanks for the initiative @EkberHasanov its look a good PR and worth to work in, as i can see the issue is regarding py37 and missing dependencies do you think its worth re-open it and work on it ? |
|
Thank you for your feedback, @yezz123 . I appreciate your positive remarks about the PR. I'm definitely interested in working on it further and ensuring that it works flawlessly without any remaining issues, but I don't know should I continue to change something and push it, because it failed so much and frustrated me. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #68 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 11 8 -3
Lines 685 552 -133
Branches 169 140 -29
==========================================
- Hits 685 552 -133 ☔ View full report in Codecov by Sentry. |
|
please review |
Co-authored-by: Akbar <98239031+EkberHasanov@users.noreply.github.com>
Kludex
left a comment
Kludex
left a comment
There was a problem hiding this comment.
I'm unsure if we should make it possible to instantiate Series with __init__.
Maybe it's better to not have the __init__, and only implement the __get_pydantic_core_schema__.
pyproject.toml
Outdated
| requires-python = '>=3.7' | ||
| dependencies = [ | ||
| 'pydantic>=2.0b3', | ||
| 'pydantic>=2.0', |
There was a problem hiding this comment.
Please open a new PR for this.
| 'pydantic>=2.0', | |
| 'pydantic>=2.0b3', |
pyproject.toml
Outdated
| all = [ | ||
| 'phonenumbers>=8,<9', | ||
| 'pycountry>=22,<23', | ||
| 'pandas==1.3.5', |
There was a problem hiding this comment.
Because pandas>=2.0 requires python >=3.8
pydantic_extra_types/pandas_types.py
Outdated
|
|
||
|
|
||
| class Series: | ||
| def __init__(self, value: Any) -> None: |
There was a problem hiding this comment.
What is this "value"? pd.Series accepts many arguments. 🤔
| ) -> core_schema.AfterValidatorFunctionSchema: | ||
| return core_schema.general_after_validator_function( | ||
| cls._validate, | ||
| core_schema.any_schema(), |
There was a problem hiding this comment.
I don't think this is the best we can do. Is it? A series should be an array of some type, and I guess we are able to get this type as well.
| black | ||
| pyupgrade | ||
| ruff | ||
| pandas-stubs |
There was a problem hiding this comment.
Likely not needed on pandas >= 2.
Oh yes, I guess inheriting the Thank you for pointing out this alternative approach. I'm excited about this implementation as it provides a more elegant and intuitive way to work with series data using Pydantic. Thank you for your feedback! |
|
@EkberHasanov how it is going? should somebody catch up and finish this PR? thank you |
Hi @dzmitry-lahoda it's all good, thanks! I have made some changes last time, but i guess i have to update library versions now. I will give it a try to update the versions of the libraries and see what is going on, and then i will let you you know if I need further help. Thanks |
|
Hi @dzmitry-lahoda, hope you're okay. I guess, I may need help to resolve all these issues. Could someone please help or perhaps explain what and where is wrong? I though I know, but it seems i'm wrong. |
4 participants
Selected Reviewer: @Kludex